[Merged by Bors] - Remove builder redundancy #3294
Conversation
…move-builder-redundancy � Conflicts: � Cargo.lock � beacon_node/execution_layer/src/engine_api.rs � beacon_node/execution_layer/src/engine_api/http.rs � beacon_node/execution_layer/src/lib.rs � beacon_node/src/cli.rs � beacon_node/src/config.rs
…ent, fix compile errors
realbigsean
added
ready-for-review
| @@ -3218,6 +3218,14 @@ impl<T: BeaconChainTypes> BeaconChain<T> { | |||
| let slot = state.slot(); | |||
| let proposer_index = state.get_beacon_proposer_index(state.slot(), &self.spec)? as u64; | |||
|
|
|||
| let pubkey_opt = match self.validator_pubkey_bytes(proposer_index as usize) { | |||
There was a problem hiding this comment.
Since you have a BeaconState in scope here, I'd suggest doing state.validators().get(proposer_index as usize).map(|v| v.pubkey) rather than hitting the pubkey cache.
There can be some lock contention on the validator pubkey cache at times. There's also something very definite about saying "this validator doesn't exist in the state of the block they're proposing". You could even return an error in that case, I don't see how any block could be valid if the proposer index isn't in the validator registry. I would only do that if there's a benefit in dropping the Option around the pubkey, otherwise we might as well just leave it a warning and have one less way that block proposal can fail.
There was a problem hiding this comment.
Ok used this state to get the pubkey here: ac15207
I've left the Option because dropping it doesn't gain us anything, but that's a good point, if the validator isn't in the state I too have no idea how it could propose. Seemed more like a possibility when I was using the cache 😂
| prev_randao: Hash256, | ||
| finalized_block_hash: ExecutionBlockHash, | ||
| suggested_fee_recipient: Address, | ||
| f: fn(&ExecutionLayer<T>, &ExecutionPayload<T>) -> Option<ExecutionPayload<T>>, |
There was a problem hiding this comment.
From what I can see this function is always a noop. Is it here for future use?
There was a problem hiding this comment.
Yes! In the fallback functionality in #3134, this populates a cache
There was a problem hiding this comment.
So it'll either noop when getting a blinded block from a relay (no way to cache the payload) or it will cache when falling back to a local EE (have to cache the fully payload and return a blinded payload becaues the VC expects a blinded payload from this endpoint)
paulhauner
added
waiting-on-author
|
Ok, I've addressed comment! |
realbigsean
added
ready-for-review
paulhauner
added
ready-for-merge
|
bors r+ |
## Issue Addressed This PR is a subset of the changes in #3134. Unstable will still not function correctly with the new builder spec once this is merged, #3134 should be used on testnets ## Proposed Changes - Removes redundancy in "builders" (servers implementing the builder spec) - Renames `payload-builder` flag to `builder` - Moves from old builder RPC API to new HTTP API, but does not implement the validator registration API (implemented in #3194) Co-authored-by: sean <[email protected]> Co-authored-by: realbigsean <[email protected]>
bors
bot
changed the title
## Issue Addressed Part of #3118, continuation of #3257 ## Proposed Changes - the [ `first_success_without_retry` ](https://github.com/sigp/lighthouse/blob/9c429d0764ed91cf56efb8a47a35a556b54a86a4/beacon_node/execution_layer/src/engines.rs#L348-L351) function returns a single error. - the [`first_success`](https://github.com/sigp/lighthouse/blob/9c429d0764ed91cf56efb8a47a35a556b54a86a4/beacon_node/execution_layer/src/engines.rs#L324) function returns a single error. - [ `EngineErrors` ](https://github.com/sigp/lighthouse/blob/9c429d0764ed91cf56efb8a47a35a556b54a86a4/beacon_node/execution_layer/src/lib.rs#L69) carries a single error. - [`EngineError`](https://github.com/sigp/lighthouse/blob/9c429d0764ed91cf56efb8a47a35a556b54a86a4/beacon_node/execution_layer/src/engines.rs#L173-L177) now does not need to carry an Id - [`process_multiple_payload_statuses`](https://github.com/sigp/lighthouse/blob/9c429d0764ed91cf56efb8a47a35a556b54a86a4/beacon_node/execution_layer/src/payload_status.rs#L46-L50) now doesn't need to receive an iterator of statuses and weight in different errors ## Additional Info This is built on top of #3294
Issue Addressed
This PR is a subset of the changes in #3134. Unstable will still not function correctly with the new builder spec once this is merged, #3134 should be used on testnets
Proposed Changes
payload-builderflag tobuilder